Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add form-validation-states Sass map #27999

Merged
merged 3 commits into from Jan 11, 2019
Merged

Add form-validation-states Sass map #27999

merged 3 commits into from Jan 11, 2019

Conversation

mdo
Copy link
Member

@mdo mdo commented Jan 8, 2019

Fixes #26759.

@mdo mdo added this to Inbox in v4.3 via automation Jan 8, 2019
@mdo mdo requested a review from a team as a code owner January 8, 2019 01:59
@mdo mdo moved this from Inbox to Needs review/changes in v4.3 Jan 8, 2019
@ysds
Copy link
Member

ysds commented Jan 9, 2019

What do you think about the validation icons?

bootstrap/scss/mixins/_forms.scss

Lines 64 to 68 in 741b6cd

@if $state == "valid" {
background-image: $form-feedback-icon-valid;
} @else {
background-image: $form-feedback-icon-invalid;
}

An idea using nested map:
https://www.sassmeister.com/gist/cbbbea890ee2bd97de9ae3b9f063729d

@ysds
Copy link
Member

ysds commented Jan 11, 2019

LGTM 👍

MartijnCuppens
MartijnCuppens approved these changes Jan 11, 2019
View reviewed changes
Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, also great suggestion by @ysds!

@MartijnCuppens MartijnCuppens moved this from Needs review/changes to Ready to merge in v4.3 Jan 11, 2019
@XhmikosR XhmikosR merged commit deea117 into v4-dev Jan 11, 2019
v4.3 automation moved this from Ready to merge to Shipped Jan 11, 2019
@XhmikosR XhmikosR deleted the validation-map branch January 11, 2019 19:16
@mdo mdo mentioned this pull request Jan 11, 2019
Closed
@ffoodd ffoodd mentioned this pull request Oct 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MartijnCuppens MartijnCuppens MartijnCuppens approved these changes

Assignees
No one assigned
Labels
css feature v4
Projects
No open projects
v4.3
  
Shipped
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mdo @ysds @MartijnCuppens @XhmikosR